SQLAlchemy: Ignore SQL's "FOR UPDATE" clause #584
Conversation
| def for_update_clause(self, select, **kw): | ||
| # CrateDB does not support currently 'FOR UPDATE' statements. | ||
| # See https://github.com/crate/crate-python/issues/577 | ||
| return '' |
There was a problem hiding this comment.
Good point about the user warning, @matriv. I've just added a corresponding improvement to the other patch with 154d77a. If you would be so kind to adjust the patch like that, @surister? 🙏
I think it will be good like this. If you insist on adding it also to the documentation, I can take this, so we do not need to bother @surister about finding a good spot ;].
There was a problem hiding this comment.
I think that logging a warning is enough, mainly for those users that quickly try out CrateDB as their backend (switching from some other DB), to be informed.
People working with CrateDB should already know it doesn't support transactions, so adding a docs entry about that to the python client is a bit too much, so I think it's fine to just log a warning.
There was a problem hiding this comment.
Thanks. I've amended the patch by adding a similar warning like on the other one.
|
Dear @surister, after bringing in #582 and #583, I am going to wrap this up, if you don't have any objections. Users are waiting for another release, and my intention was only to bring in a few small maintenance updates and little improvements like this. Your contribution was a perfect fit for that, and arrived just in time. Thank you again. With kind regards, |
amotl
force-pushed
the
ignore_for_update
branch
2 times, most recently
from
49fb8c2 to
3ba3bc8
Compare
amotl
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
amotl
force-pushed
the
ignore_for_update
branch
from
3ba3bc8 to
7e35438
Compare
amotl
force-pushed
the
ignore_for_update
branch
from
7e35438 to
44c21d2
Compare
amotl
force-pushed
the
ignore_for_update
branch
from
44c21d2 to
c07e203
Compare
Summary of the changes / Why this is an improvement
Pretty straightforward, now queries like
select(table).with_for_update()will not generate aFOR UPDATEstatement.Resolves crate/sqlalchemy-cratedb#95.
Checklist